Conversation
|
@jihyeonjang The fix looks good to me. Would you be willing to add more detail to the commit message (the PR description is fine)? I think it's helpful when looking back through the commit log years after commits were made to be able to get a better idea of exactly what was changed in a commit based on the log entry. The information from the PR description would probably work well if integrated into the commit message. If you have any questions about amending a commit, just let me know and I'll be glad to help. |
jihyeonjang
force-pushed
the
bugfix_evapprod_develop
branch
2 times, most recently
from
ee6a49a to
bc42b1d
Compare
|
@mgduda Thank you for reviewing this PR. I added the commit message, similar to the PR description. Please let me know if anything should be revised. Thanks! |
mgduda
left a comment
mgduda
left a comment
There was a problem hiding this comment.
Thanks so much for reworking the commit message -- this looks great!
|
@jihyeonjang Actually, although the commit message suggests the fix is based on the commit where the typo was introduced (i.e., 8e00576), it looks like this PR branch is actually based on 447de92 . If you don't object, I can rebase this PR branch onto 8e00576 before merging. |
|
Sure. That would be better. Thank you for checking the details. |
After rebasing the changes in this branch onto 8e00576, I think the commit message should still be correct. |
This fixes a typo in src/core_atmosphere/physics/mpas_atmphys_interface.F where evapprod(k,k) was used instead of evapprod(k,i), introduced in 8e00576. The fix corrects diagnostic output, but should not affect the model state or simulation results. The change is based on the commit where the typo was introduced, so it can be applied cleanly to older MPAS versions without unrelated changes.
mgduda
force-pushed
the
bugfix_evapprod_develop
branch
from
bc42b1d to
c628836
Compare
2 participants
This PR fixes a typo in src/core_atmosphere/physics/mpas_atmphys_interface.F where evapprod(k,k) was used instead of evapprod(k,i) (introduced in 8e0057689d).
This issue was reported in Issue #1395.
This corrects the diagnostic output and should not affect model results.
I based the fix on the commit where the typo was introduced, so it can be applied to older MPAS versions without bringing in unrelated changes.